Skip to content

fix(metadata-protocol): listDrafts emits the ISO-8601 string updatedAt declares - #16452

Merged
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-14938-listdrafts-updatedat-canonical-iso
Sep 7, 2026
Merged

fix(metadata-protocol): listDrafts emits the ISO-8601 string updatedAt declares#16452
zhuangjianguo merged 2 commits into
mainfrom
claude/issue-14938-listdrafts-updatedat-canonical-iso

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #14938

SysMetadataRepository.listDrafts declares updatedAt: string | null and emitted the raw updated_at column, so on Postgres and MySQL it handed a JS Date through a field its own signature calls a string. Routed through the file's existing canonicalIsoInstant, with the terminal chosen per call site: null here.

Authored by Claude Code in session session_01ARYe3yQTQCUFm5qPYNgKaJ, dispatched from the domain:engine PM seat.

The defect, and why it was invisible twice over

The declaration is an inline TypeScript return type on the method, not a Zod schema — so the schema search #13973's census ran comes up empty. And rows is cast as any[] one line above the map, so tsc saw a string assignment that never happened. Two independent reasons, both named on the card.

updated_at / created_at are the builtin audit columns. SqlDriver#formatOutput repairs them (repairNaiveUtcAuditTimestamp over AUDIT_TIMESTAMP_COLUMNS) and folds the declared datetime columns only inside its if (this.isSqlite) arm, and withPostgresCalendarDayAsText leaves timestamptz / timestamp deliberately untouched because those are instants.

Measured at the published door, both legs

Built @objectstack/metadata-protocol at this head, swapped the one edited source file back to the merge base ac76425f0, rebuilt, and drove dist/index.js with a hand-made Date — the shape the live dialects produce:

leg typeof value conforms to the published declaration
base ac76425f0 object (a Date) Wed Mar 04 2026 05:06:07 GMT+0000 (Coordinated Universal Time) false
this head string 2026-03-04T05:06:07.089Z true

Restore proven byte-exact (blob equals the HEAD blob, git diff HEAD empty), and the head bundle rebuilt byte-identically afterwards. Rebuild liveness is not assumed: dist/index.js mtimes moved on every leg.

Why the terminal is null here and not rowToItem's ?? new Date(...)

The terminal is chosen per call site under the #14078 ruling. This projection declares updatedAt: string | null and the chain being replaced already ended in ?? null, so null is what "absent" already means to every consumer of this list — synthesising a "now" would invent an edit instant for a row that never recorded one. An Invalid Date (reachable on both live dialects, measured in #14409) takes that same branch through the total Date arm.

Scope held deliberately narrow

Test, with a proven-red ablation

packages/metadata-protocol/src/sys-metadata-repository-14938-list-drafts-updated-at.test.ts — 8 cases across five sections: the Date on updated_at, the same through the org-scoped $or read, the created_at fallback, SQLite's already-canonical string passing through byte-identically, the two terminal cases (absent columns and an Invalid Date), and updatedBy staying as it is.

The conformance table is keyed by keyof DraftHeader, and DraftHeader is not a hand-copied shape: it is derived in the test file from the method's own signature, by taking Awaited of its ReturnType and indexing that array type by number. So a field added to or removed from that inline return type reddens this file at type-check time rather than going unchecked. Every case guards non-vacuity with toBeInstanceOf(Date) on the seeded row before reading the output.

Ablation on the pre-fix line, whole-line anchor, mutation proven on disk before measuring (git hash-object moved away from the HEAD blob; pre-fix text present exactly once, fixed text zero times):

mutated (pre-fix)  ->  6 failed | 2 passed (8)
restored           ->  8 passed (8)

The 2 that survive the mutation are the SQLite idempotence case and the both-columns-absent case — the two the pre-fix expression already satisfied. Restore proven byte-exact in the same shell.

Verification, all at 9d12b684a on a clean tree

  • pnpm --filter @objectstack/metadata-protocol typecheck — 0 errors, and tsc --listFiles shows the new pin test inside the 724-file program, so this is not a green about a file the config excludes.
  • pnpm --filter @objectstack/metadata-protocol test — 169 files passed, 2 skipped; 2432 tests passed, 10 skipped.
  • All 63 gate families derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack for this change set, run and green. The list was re-derived after the ledger row landed, which added 8 families the first derivation did not name.
  • pnpm lint (eslint . --no-inline-config, whole repo) — exit 0. No narrowing claimed.
  • pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' — 71/71, so check:dts-closure (71 built packages, 163/163), check:dual-build-cjs-loads and check:type-check-debt --re-measure (5 ledger entries, none above its recorded number) are measurements rather than PREREQUISITE-NOT-MET skips.

check:engine-double-contract asked for the new fake-engine double to be registered in its shrink-only pinned ledger; --write recorded three verb rows, 3 added or grown, 0 lost.

Bump level

patch. No public surface moves: across the ablation all 24 files under dist/ were compared by sha256 and dist/index.d.ts and dist/index.d.cts are byte-identical, already declaring updatedAt: string | null on both legs. Only index.js, index.cjs and their two sourcemaps differ, and the whole runtime delta is the one expression plus the comment block above it. Per the Check Changeset prose in pr-automation.yml, a fix( that changes no public surface stays patch — which is also what the two in-package precedents for this act took (#14040 and #16427, both patch, neither bannered).

No BREAKING banner is owed: nothing is removed or renamed, no required member is added, and no accept set narrows — the door's parameter is untouched and nothing that was accepted is now refused.


Generated by Claude Code

…ter boundary

`SysMetadataRepository.listDrafts` declares `updatedAt: string | null` on an
inline TypeScript return type and reached the field through
`row.updated_at ?? row.created_at ?? null`. `??` fires only on nullish, so the
JS `Date` that Postgres and MySQL materialise for the builtin audit columns
walked straight past it into a field declared a string.

Route the value through the file's existing `canonicalIsoInstant`, the same
producer-side canonicalisation `rowToItem` applies, with the terminal chosen per
call site: `null` here, because the chain being replaced already ended in
`?? null` and that is what "absent" already means to this projection's
consumers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
check:engine-double-contract requires every findOne/update/delete fake engine
double in a test file to be registered in the pinned, shrink-only ledger. The
new #14938 pin test carries one; `--write` recorded its three verb rows —
3 added or grown, 0 lost — and nothing else moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/m label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 2 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via SysMetadataRepository (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 73d30a4d87361dc5fdec11b7e4dbd27cdb672e4d — the merge of head 9d12b684ad46c3d5253cd8a8d2ed924f601b27d2 into base 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 73d30a4d87361dc5fdec11b7e4dbd27cdb672e4d && git checkout 73d30a4d87361dc5fdec11b7e4dbd27cdb672e4d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3a 9d12b684ad46c3d5253cd8a8d2ed924f601b27d2 && git checkout -B drift-repro 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3a && git merge --no-ff 9d12b684ad46c3d5253cd8a8d2ed924f601b27d2

node scripts/docs-audit/affected-docs.mjs --json 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 6eba38f5a35a8c562f39f1c2c3d8d53f41d04b3a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 7, 2026
@claude

claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

PM ruling on the flagged Zone-1 boundary call: your judgement stands, keep the hunk. And no contract review is required here — that is a decision with a reason, stated below.

domain:engine dispatching seat, session session_01ARYe3yQTQCUFm5qPYNgKaJ, 2026-09-07T02:1xZ.

The boundary call — upheld

The round asked, rather than deciding silently, whether correcting one clause of canonicalIsoInstant's docblock violates the dispatch's Zone 1.2 ("⛔ this card must NOT touch the isoFromValidDate / canonicalIsoInstant consolidation"). The sentence read "both callers (getByHash and rowToItem) already end in ?? new Date(...).toISOString()", and this fix makes it false by adding a third caller with a different terminal.

That is the right call, and asking was the right way to make it. Zone 1.2 exists to fence off #16422 — the consolidation of the two helpers into one spelling, which is a maintainer decision that has been put to the maintainer and is unanswered (its blocker is that MetadataHistoryRecord.recordedAt, a REQUIRED z.string().datetime(), has no terminal value either half of the #14078 ruling supplies). A caller census that your own diff falsifies is not that consolidation. It renames nothing, moves nothing, exports nothing, collapses nothing, and #16422's paragraph is byte-untouched.

And the alternative was worse in a way this repo has already paid for: shipping a knowingly-false sentence into the tree is the same class of defect as the #16296 changeset that said "A NON-BREAKING ADDITION" about an accept-set narrowing — text that is wrong at the moment it lands, and that the next reader has no way to distrust. ⛔ A fence against re-documenting a family is not a licence to leave false documentation behind.

Why this PR gets no contract review — and why that is not a shortcut

The round declared Clause-②: no. This seat has now called the tier wrong three times in one session, so the declaration was not taken on trust. What settles it:

probe reading
limb 1 dist/index.d.ts and dist/index.d.cts byte-identical across the two-leg ablation — and both already declare updatedAt: string | null at base as well as head. Nothing on the published surface moves in either direction
limb 2 answered by RUNNING the published door on both legs, ⛔ not by reading types — base emitted a Date through a declaration the same build spelled string | null; head emits '2026-03-04T05:06:07.089Z'
② carriers check-clause2-carriers --pair 16452 REAL_EXIT=0, re-run by this seat from a worktree at origin/main 6eba38f5a with checker blob 751b4a6e4 == origin/main's

⇒ this is the removal of a false emission — a producer brought into conformance with a type it had already published — not an accept-set narrowing. A TypeScript consumer could never have been written against the Date, because the .d.ts said string | null on both legs.

And the precedents were verified by this seat, by content, not taken from the report. Both named precedents are the same package and the same act, and both shipped patch with zero **BREAKING markers:

precedent changeset level **BREAKING
#14040 a7002ce5a .changeset/lucky-pugs-shave.md @objectstack/metadata-protocol: patch 0
#16427 3e7ef9c23 .changeset/invalid-date-total-arm-metadata-protocol.md @objectstack/metadata-protocol: patch 0

That is consistent with .github/workflows/pr-automation.yml:667-674: the act decides the level, and an act that moves no public surface stays patch.

What the round proved that the dispatch only assumed

  • 2.2 became mechanical rather than argued. updated_by/created_by are Field.lookup('sys_user'), and the driver's repair set is AUDIT_TIMESTAMP_COLUMNS = ['created_at', 'updated_at'] — exactly two members, neither a *_by column — so the dialect asymmetry cannot reach updatedBy. Corroborated at the door: updatedBy is string on both legs. Pinned in §E so the next reader need not re-derive it.
  • 2.3 was checked exhaustively, not spot-checked. sys_metadata declares exactly two Field.datetime columns, and every read of either now routes through canonicalIsoInstant (:1199 this fix, :1897 repaired by MetadataItem.authoredAt is declared z.string() but receives a JS Date on Postgres/MySQL — a silent declared-contract violation, because the schema is parsed only in its own test #13997); the only other hit is a write. ⇒ there is no second Date-into-declared-string site on the list() / rowToItem header projection, so no second card and no widened PR.
  • 2.1 was falsified in the dispatch's favour — an outcome worth naming because it is the rarer direction: the card's line numbers (:1140/:1108, measured at 431979e67) are stale, but the dispatch's re-read (:1178/:1148) was still exact. The round re-derived by symbol anyway and proved the whole-line anchor unique in both directions before mutating.

Remaining before release

⛔ Condition ③ only: CI is 17/30 at the time of writing. ⛔ Not a required-checks subset and ⛔ not a fixed check count — the sibling PRs in this batch moved from 37/37 to 40/40 while they sat, which is exactly why the rule is completed == total.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 7, 2026 04:58
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit c383352 Sep 7, 2026
35 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14938-listdrafts-updatedat-canonical-iso branch September 7, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants